-
Notifications
You must be signed in to change notification settings - Fork 8.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix rendering of DBCS characters when partially off screen #8438
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems sensible to me, but let's make sure @miniksa agrees
Oof. I'm 80% sure that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this seems fine. I'm pretty sure the restriction was some part of me comparing what conhostv1
looked like to try to prevent it from striking the left most column weirdly.
But the longer I go for anything DBCS, the more I realize conhostv1
wasn't correct ever. So it's not really a model to stick to.
So given that the manual tests for this prove it's better and maintains the previous fix, I'm good with it.
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
🎉 Handy links: |
When the renderer is called on to render part of a line starting halfway
through a DBCS character (as can occur in conhost when the viewport is
offset horizontally), it could result in the character not being
displayed, and/or with following the characters drawn in the wrong
place. This PR is an attempt to fix those problems.
The original code for handling the trailing half of fullwidth glyphs was
introduced in PR #4668 to fix issue #2191.
When the content being rendered starts with the trailing half of a DBCS
character, the renderer tries to move the
screenPoint
back a position,so it can instead render the full character, but instructing the render
engine to trim off the left half of it.
If the X position was already in column 0, though, it would instead move
forward one position, intending to skip that character. At best this
would mean the half character wouldn't be rendered, but since the
iterator wasn't incremented correctly, it actually just ended up
rendering the character in the wrong place.
The fix for this was simply to drop the check for the X position being
in column 0, and allow it go negative. The rendering engine would then
just start rendering the character partially off screen, and only the
second half of it would be displayed, which is exactly what is needed.
The second problem was that the code incrementing the iterator was using
the
columnCount
variable rather than theit->Columns()
value for thecurrent position. When dealing with the trailing half of a DBCS
character, the
columnCount
is 2, but theColumns()
value is 1. Ifyou use the former rather than the later, then the renderer may skip the
following character.
Validation Steps Performed
I've developed a more easily reproducible version of the test case
described in #8390, and confirmed that the problem no longer occurs when
this PR is applied.
I've also manually confirmed that the problem described in #2191 that
was fixed by PR #4668 is still working correctly now.
Closes #8390